-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
90 average calibration #107
Conversation
…operly due to the issue with mljflux.
…operly due to the issue with mljflux.
…m/JuliaTrustworthyAI/LaplaceRedux.jl into 90-average-calibration-in-utilsjl
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor things (see individual comments)
.github/workflows/CI.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change necessary? I think the existing approach should use the latest release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uhm i thought i removed julia version 1.0 and left 1.10. how do i fix this? replace 1.9 with 1.10?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, nvm, let's keep as is now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this qmd needs to be re-rendered one more time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes they are not rendered yet because i am still working on them. anyway, now that we are at it, i have a question. whenever i type quarto render tutorials/etc i end up with a lot of files that they are not used. in particular i get 2 types of markdown files and other stuff that i have to delete manually. is there a way to avoid this issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, add this to _quarto.yml
render:
- "!docs/src/*.md"
src/calibration_functions.jl
Outdated
end | ||
|
||
@doc raw""" | ||
empirical_frequency_classification(y_binary, distributions::Distributions.Bernoulli) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these docstrings rendering correctly in the REPL when you use help mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i will check one by one later. at the moment i am working on the multi.qmd file. i am failing to make to join the plots in a 2,2 grid. I will upload as it is now , so that you can see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw i was considering deleting the original functions for sampled distributions and leaving only those for normal distributions since laplaceredux only works for this kind of distr. i will submit the general ones to the guys at calibration.jl @pat-alt
.github/workflows/CI.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, nvm, let's keep as is now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, add this to _quarto.yml
render:
- "!docs/src/*.md"
_quarto.yml
Outdated
@@ -2,6 +2,9 @@ project: | |||
title: "LaplaceRedux.jl" | |||
execute-dir: project | |||
|
|||
render: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pat-alt i tried placing this like piece of code both here and in /docs/_metadata.yml but it doesn't work. if i run quarto render . from inside the /tutorials folder i get 44 new files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs to be indented to fall under the project settings:
project:
title: "{{{PKG}}}.jl"
execute-dir: project
render:
- "!docs/src/*.md"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by "it stops working completely"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm found the solution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pat-alt the documentation says that when you use the prefix ! "you need to start by specifying everything you do want to render. For example: project:
render:
- "*.qmd"
- "!ignored.qmd"
- "!ignored-dir/"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pat-alt is there a way to preview the website? i would like to check if everything is okay but there doesn't seem to be a command. Seems like i am missing a step... once i render the .qmd files, how do i rerender the html part?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is handled by Documenter.jl, simply run julia --project=docs docs/make.jl
in the command line. This will render to the docs/build
directory. In there, simply click on the index.html
file, which will open the (local version of the) website in your browser
Let me know when you're happy for this to be merged, then I'll review one more time @pasq-cat |
try this version pls. i had to add seed to the arguments of the functions in data.jl. see if it's ok for you. before merging tag me because the guidelines says that the pull request must have a good description of the work done, so i have to edit title and description |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
Small comments mostly about typos/grammar.
One thing I noticed is the Calibration_plot
function name. Can we just make this calibration_plot
please? I guess that's in TaijaPlotting though
This pull request add the following things: